Add debug logging option #109
Conversation
lib/cc/engine/duplication.rb
Outdated
| Dir.chdir(directory) do | ||
| languages_to_analyze.each do |language| | ||
| if engine_config.debug? | ||
| $stderr.puts("Analyzing language #{language}") |
There was a problem hiding this comment.
This seems to be the only log line that can't use Reporter#debug and has to duplicate the logic. Thoughts on just ditching it? Seems low value to me.
There was a problem hiding this comment.
Ditching it sounds good to me.
|
LGTM. Note that to do CLI testing, you'll need to make a change there to display engine STDERR even in the successful case (it doesn't do this currently). |
Right. I've been running the engine directly with Do you think this is useful enough generally to make that CLI change? |
dblandin
force-pushed
the
devon/add-debug-logging
branch
from
b748419 to
433c117
Compare
lib/cc/engine/analyzers/reporter.rb
Outdated
| sexp = language_strategy.run(file) | ||
| process_sexp(sexp) | ||
|
|
||
| processed_files_count += 1 |
There was a problem hiding this comment.
Do you need anything here to handle the concurrency and incrementing this number?
There was a problem hiding this comment.
I don't think so. The join below should block until all threads return.
There was a problem hiding this comment.
Right but this block is executed inside the thread right? So more than one thread could try to increment this at once
There was a problem hiding this comment.
Gotcha, I suppose that's true! I'll wrap the increment with a mutex.
There was a problem hiding this comment.
You could also use this class from concurrent-ruby I think http://ruby-concurrency.github.io/concurrent-ruby/Concurrent/AtomicFixnum.html
There was a problem hiding this comment.
Nice! That's handy!
Nope, just letting you know about a gotcha that I've hit before. Whoever thinks it's useful should make that change. |
This PR adds a configuration option for debug logging in order to shake out some difficult to diagnose analysis issues.
@codeclimate/review 🔎